Skip to content

refactor: code quality cleanup pass#8

Open
ADWilkinson wants to merge 1 commit intomainfrom
cleanup/code-quality
Open

refactor: code quality cleanup pass#8
ADWilkinson wants to merge 1 commit intomainfrom
cleanup/code-quality

Conversation

@ADWilkinson
Copy link
Copy Markdown
Owner

Summary

Combined code-quality cleanup pass across all 8 review areas. Single PR because the repo is ~2400 LOC — full breakdown in CLEANUP_ASSESSMENT.md.

Dedup / DRY

  • New src/steps/shared.ts with loadPromptTemplate, readClaudeMd, getPluginFlag, codexEffortConfig, review model/effort helpers. Previously duplicated across plan.ts, execute.ts, pr.ts, review.ts.
  • Collapsed buildRemoteCommandParts + buildLocalChildArgs into a single buildPipelineArgs({ escape }).

Weak types

  • Replaced e.step! / e.label! / e.elapsed! in stats.ts with isStepDone / isStepFailed type guards — non-null assertions gone, narrowed types visible.

AI slop

  • Dropped a few restating comments in pipeline.ts and pr.ts. Kept every real WHY comment (graceful degradation, rebase race, contract header, plugin-dir JSDoc).

No action needed

  • Type consolidation: no duplicated shapes.
  • Dead code: knip flagged ONESHOT_STEPS, getStepShort, event types, ExecResult, etc. — all are the documented public contract mirrored by oneshot-bot/src/services/oneshot-contract.ts. KEEP.
  • Circular deps: madge --circular reports none.
  • Defensive try/catch: every catch is a legitimate boundary (file I/O, worktree teardown, best-effort event writes, Linear update, fail-safe classification, top-level CLI).
  • Legacy / deprecated: none.

Test plan

  • bun run typecheck
  • bun test (16 pass)
  • bun run build
  • madge --circular still clean (20 modules)

Combined cleanup covering the 8 quality areas (repo is ~2400 LOC so a
single PR keeps the diff reviewable).

- dedup: extract loadPromptTemplate, readClaudeMd, getPluginFlag,
  codexEffortConfig, review model/effort helpers into steps/shared.ts
  (previously duplicated across plan, execute, pr, review)
- dedup: collapse buildRemoteCommandParts and buildLocalChildArgs into
  a single buildPipelineArgs({ escape }) helper
- weak types: replace e.step! / e.label! / e.elapsed! in stats.ts with
  isStepDone / isStepFailed type guards
- ai slop: drop a handful of restating comments in pipeline.ts and pr.ts;
  keep all WHY-comments (graceful degradation, rebase race, contract header)

Assessment doc at CLEANUP_ASSESSMENT.md. No behaviour changes, no public
API changes, knip surface preserved (published as source-level contract
for oneshot-bot). Gate green: typecheck + 16 tests + build.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant